Skip to content

build: upgrade rusqlite 0.30.0 → 0.31.0#6219

Merged
mhammond merged 1 commit intomozilla:mainfrom
erichdongubler-mozilla:rusqlite-0.31
May 29, 2024
Merged

build: upgrade rusqlite 0.30.0 → 0.31.0#6219
mhammond merged 1 commit intomozilla:mainfrom
erichdongubler-mozilla:rusqlite-0.31

Conversation

@ErichDonGubler
Copy link
Contributor

@ErichDonGubler ErichDonGubler commented Apr 25, 2024

This PR is intended to unblock duplicate dependencies that would be introduced as new versions of ahash and hashbrown in Firefox bug 1893057.

Pull Request checklist

  • Breaking changes: This PR follows our breaking change policy
    • This PR follows the breaking change policy:
      • This PR has no breaking API changes, or
      • There are corresponding PRs for our consumer applications that resolve the breaking changes and have been approved
  • Quality: This PR builds and tests run cleanly
    • Note:
      • For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
      • If this pull request includes a breaking change, consider cutting a new release after merging.
  • Tests: This upgrades what I presume is a well-vetted dependency (though I'm not familiar with it). Assuming coverage in CI, I'll leave it to maintainer judgment on whether testing is sufficient.
  • Changelog: Will address this shortly.
  • Dependencies: This PR follows our dependency management guidelines
    • Any new dependencies are accompanied by a summary of the due dilligence applied in selecting them.

Branch builds: add [firefox-android: branch-name] to the PR title.

@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 22.73%. Comparing base (33ea536) to head (9a546c1).

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #6219       +/-   ##
===========================================
- Coverage   50.85%   22.73%   -28.12%     
===========================================
  Files         112      330      +218     
  Lines       11811    29705    +17894     
===========================================
+ Hits         6006     6753      +747     
- Misses       5805    22952    +17147     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ErichDonGubler
Copy link
Contributor Author

RE: the failing Dependency Check / check-dependencies (3.7) (pull_request): This doesn't seem related this to PR. 🤔 I'm not positive, though.

@mhammond
Copy link
Member

I opened #6221 for the dep-check failure.

@linabutler
Copy link
Contributor

Rebasing to pick up #6221; let's merge if it's green!

@linabutler
Copy link
Contributor

@ErichDonGubler Thanks for the patch! It looks like the dependency summary might need updating—could you please run:

python ./tools/dependency_summary.py > DEPENDENCIES.md

It looks like zerocopy is an optional dependency of ahash, and allocator-api2 is an optional dependency of hashbrown, and they're both license-compatible.

Copy link
Contributor

@linabutler linabutler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the dependency summary is updated (#6219 (comment)), this should be good to go!

@ErichDonGubler
Copy link
Contributor Author

ErichDonGubler commented May 2, 2024

@linabutler: I promise I will do the dependency summary re-generation! I've ben holding off on bringing this out of draft and handling feedback until I've dealt with a few things that are outstanding for making landing in mozilla-central easy. This effort has several fronts:

  • Land patches for Firefox's bug 1893057 to make it possible to incrementally update hashbrown. This just landed, and I think it's going to stick the landing. 🤞🏻🎉 EDIT: It stuck!
  • kyren/hashlink#28 should eliminate the allocator-api2 dependency that @glandium and I have been fighting to keep out-of-tree.

Once these are in place, I think this should be good to go. I'm happy to consult on a patch to consume this on the Firefox end, if that helps!

@linabutler
Copy link
Contributor

@ErichDonGubler Great to hear that the m-c hashbrown bump stuck! No rush at all on this PR; I just requested changes to tidy up my review queue 😊

@ErichDonGubler
Copy link
Contributor Author

ErichDonGubler commented May 8, 2024

@linabutler: While hashlink has not yet had a release after the PR I was hoping to land got merged, I decided to try out the instructions for regenerating DEPENDENCIES.md. I ran into this after rebasing onto main (482a9fe):

python ./tools/dependency_summary.py > DEPENDENCIES.md
    Updating git repository `https://github.com/martinthomson/ohttp.git`
  Downloaded redox_termios v0.1.2
  Downloaded wasm-bindgen-backend v0.2.81
  Downloaded wasm-bindgen-shared v0.2.81
  Downloaded x11-clipboard v0.7.1
  Downloaded wasm-bindgen-macro-support v0.2.81
  Downloaded wasm-bindgen-macro v0.2.81
  Downloaded term v0.6.1
  Downloaded wasm-bindgen v0.2.81
  Downloaded openssl-sys v0.9.90
  Downloaded openssl v0.10.55
  Downloaded web-sys v0.3.58
  Downloaded security-framework v2.6.1
  Downloaded nix v0.24.1
  Downloaded garando_syntax v0.1.0
  Downloaded cxx v1.0.92
  Downloaded linux-raw-sys v0.3.2
  Downloaded garando_pos v0.1.0
  Downloaded garando_errors v0.1.0
  Downloaded cxxbridge-macro v1.0.92
  Downloaded ctest2 v0.4.7
  Downloaded wasm-bindgen-futures v0.4.31
  Downloaded cxxbridge-flags v1.0.92
  Downloaded bumpalo v3.10.0
  Downloaded neli v0.6.4
  Downloaded js-sys v0.3.58
  Downloaded jni v0.20.0
  Downloaded cxx-build v1.0.92
  Downloaded termion v1.5.6
  Downloaded smithay-clipboard v0.6.6
  Downloaded security-framework-sys v2.6.1
  Downloaded numtoa v0.1.0
  Downloaded neli-proc-macros v0.1.3
  Downloaded openssl-src v111.25.0+1.1.1t
  Downloaded 33 crates (8.8 MB) in 8.23s (largest was `openssl-src` at 5.1 MB)
Traceback (most recent call last):
  File "E:\mozilla\firefox\fx-application-services\tools\dependency_summary.py", line 1486, in <module>
    print_dependency_summary_markdown(deps, file=output)
  File "E:\mozilla\firefox\fx-application-services\tools\dependency_summary.py", line 1387, in print_dependency_summary_markdown
    sections = group_dependencies_for_printing(deps)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "E:\mozilla\firefox\fx-application-services\tools\dependency_summary.py", line 1311, in group_dependencies_for_printing
    for info in deps:
  File "E:\mozilla\firefox\fx-application-services\tools\dependency_summary.py", line 1053, in get_dependency_summary
    yield self.get_license_info(id)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "E:\mozilla\firefox\fx-application-services\tools\dependency_summary.py", line 1167, in get_license_info
    "license_text":  self._fetch_license_text(id, licenseFile, pkgInfo),
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "E:\mozilla\firefox\fx-application-services\tools\dependency_summary.py", line 1231, in _fetch_license_text
    r.raise_for_status()
  File "C:\Users\Erich\AppData\Roaming\Python\Python311\site-packages\requests\models.py", line 1021, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://raw.githubusercontent.com/thomcc/rust-base16/master/LICENSE-CC0

It seems like the repository is being queried for this license file, and that path no longer exists. Okay, the error makes sense, given what we're trying to do. But…what the script is doing actually doesn't make sense to me. 😅 Shouldn't we be using files from the Cargo dependency packages themselves?

Anyway, this seems like a blocker for running the script. 🫤

@mergify mergify bot dismissed linabutler’s stale review May 8, 2024 00:30

The pull request has been modified, dismissing previous reviews.

@mhammond
Copy link
Member

mhammond commented May 8, 2024

Looks like that specific issue might be #6230

@ErichDonGubler ErichDonGubler marked this pull request as ready for review May 22, 2024 15:19
@ErichDonGubler ErichDonGubler force-pushed the rusqlite-0.31 branch 2 times, most recently from 8ae5d15 to 7806d64 Compare May 22, 2024 15:45
@ErichDonGubler
Copy link
Contributor Author

Rebased, and finally Ready for review! The swiftlint job keeps failing, but that seems orthogonal to this PR?

@mhammond
Copy link
Member

The swift failure does look orthogonal, but the dependency check looks related to this.

@ErichDonGubler
Copy link
Contributor Author

ErichDonGubler commented May 22, 2024

@mhammond: Yeah, I've been force-pushing so much in the last few hours because of that check. I needed to run more of the steps captured in tools/regenerate_dependency_summaries.sh after following @linabutler's instructions. The dependency check should be green now, with my latest (of multiple) force-pushes.

I had a slow feedback loop with CI to resolve this; running these steps locally introduces winapi as a dependency in android and ios, which…doesn't make sense to me (noting that I am executing these Python scripts on Windows, ATM). More pertinently, winapi is not present in the output that CI tests against. So, I either needed to generate things myself locally, and remove what CI tells me to, or to simply make the changes that CI tells me with no local changes to start. 😓

@mhammond
Copy link
Member

noting that I am executing these Python scripts on Windows, ATM

Yeah - this is a known footgun - sorry it's so painful, but it lgtm now, thanks!

@mhammond
Copy link
Member

I guess you want to coordinate landing this with that desktop bug? Let us know when you require that "merge" button hit!

@ErichDonGubler
Copy link
Contributor Author

@mhammond: That bug is resolved, so no pressure there (and no hard blockers either, AFAICT). The only consideration I can see at this point is understanding that a cargo vet audit of hashlink 0.8.2 → 0.9.1 will be necessary the next time y'all re-vendor code into mozilla-central. I can trivially provide a delta audit of 0.9.0 → 0.9.1, but I'm less familiar with the 0.8.* series of changes.

@mhammond mhammond added this pull request to the merge queue May 29, 2024
Merged via the queue into mozilla:main with commit 3ca0676 May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants